Skip to content

Conversation

@kinkard
Copy link
Contributor

@kinkard kinkard commented Apr 7, 2025

This PR disables serde serialization and deserialization for OSM objects by default, reducing compiled binary size from 1.3MB to 1.1 MB and improving compilation times.
This serialization and deserialization (#[derive(Serialize, Deserialize)]) can be enabled via serde feature by either:

  • cargo add osmpbfreader -F serde
  • osmpbfreader = { version = "0.19", features = ["serde"]} in Cargo.toml

@kinkard kinkard changed the title feat: Make serde serialization and deserialization optional feat: Make possible to disable serde serialization and deserialization via default-features = false Apr 8, 2025
@kinkard kinkard force-pushed the possibility-to-disable-serde branch from 26c6b0b to c3deca7 Compare April 8, 2025 06:58
Copy link
Owner

@TeXitoi TeXitoi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great!

Please update version to 0.19.0 as this is a breaking change

@kinkard
Copy link
Contributor Author

kinkard commented Apr 8, 2025

Great!

Please update version to 0.19.0 as this is a breaking change

Well, the current implementation with the default = ["serde"] will actually keep previous behaviour. What do you think about changing it to "disable by default" (so an explicit -F serde or features = ["serde"] would be required) to really motivate the version increase? ;)

@TeXitoi
Copy link
Owner

TeXitoi commented Apr 8, 2025

That’s just technical: is someone has default_features=false and use serde, it will break.

I’m not affraid on upgrading the minor version on this crate, that’s already 18!

@TeXitoi
Copy link
Owner

TeXitoi commented Apr 8, 2025

Ha, I have your point, ok to disable by default.

@kinkard kinkard force-pushed the possibility-to-disable-serde branch from c3deca7 to 9b51e31 Compare April 8, 2025 21:43
…e` feature

Disables serde serialization and deserialization for OSM objects by default.
This serialization and deserialization (`#[derive(Serialize, Deserialize)]`)
can be enabled via `serde` feature by either:
- `cargo add osmpbfreader -F serde`
- `osmpbfreader = { version = "0.19", features = ["serde"]}` in `Cargo.toml`
@kinkard kinkard force-pushed the possibility-to-disable-serde branch from 9b51e31 to 0a5de06 Compare April 8, 2025 21:45
@kinkard kinkard requested a review from TeXitoi April 8, 2025 21:46
@kinkard kinkard changed the title feat: Make possible to disable serde serialization and deserialization via default-features = false feat: Make serde serialization and deserialization optional via serde feature Apr 9, 2025
Copy link
Owner

@TeXitoi TeXitoi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it could be great to test with and without the feature in the CI, at least to be sure that it compiles.

@TeXitoi TeXitoi merged commit f83f96f into TeXitoi:master Apr 9, 2025
1 check passed
@TeXitoi
Copy link
Owner

TeXitoi commented Apr 9, 2025

v0.19.0 published, thanks!

@kinkard kinkard deleted the possibility-to-disable-serde branch April 9, 2025 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants